fix(exceptions): prevent info leak from exception cause chain#366
Merged
fix(exceptions): prevent info leak from exception cause chain#366
Conversation
Contributor
Cairn Quality ReportCommit:
|
…oError output Security fix: Exception cause messages could leak sensitive data like passwords, tokens, connection strings, etc. Changes: - Added _SENSITIVE_PATTERNS regex list for common sensitive patterns - Added _redact_sensitive() helper to redact matched patterns - Updated PhloError._format_message() to redact sensitive data in cause - Added test to verify redaction works correctly Preserves diagnostic value for non-sensitive errors while protecting against credential leaks in error output.
iamgp
force-pushed
the
fix/support-correctness-batch-348
branch
from
3af7b1a to
c54ec98
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| r"\b(password|passwd|token|secret|api_key|apikey|credential)\b\s*[:=]\s*[^\s,;]+", | ||
| re.IGNORECASE, | ||
| ) | ||
| _AUTHORIZATION_SENSITIVE_PATTERN = re.compile(r"\b(authorization|bearer)\b\s+\S+", re.IGNORECASE) |
There was a problem hiding this comment.
🟡 _AUTHORIZATION_SENSITIVE_PATTERN over-redacts common error context words after "authorization"
The _AUTHORIZATION_SENSITIVE_PATTERN regex \b(authorization|bearer)\b\s+\S+ unconditionally redacts whatever single word follows "authorization" or "bearer". This corrupts common error messages where these words are followed by non-sensitive context like "failed", "denied", "required", or "header".
Examples of false-positive redaction
"authorization failed"→"authorization <redacted>"("failed" is not a secret)"authorization denied"→"authorization <redacted>""authorization required"→"authorization <redacted>""missing authorization header"→"missing authorization <redacted>""authorization error: invalid credentials"→"authorization <redacted> invalid credentials"
These are common error message patterns from HTTP clients and auth libraries. When such exceptions are wrapped as a cause in a PhloError, the diagnostic context is destroyed, making it significantly harder to debug authorization-related failures.
Prompt for agents
The _AUTHORIZATION_SENSITIVE_PATTERN at line 14 of src/phlo/exceptions.py matches any word following 'authorization' or 'bearer', causing false-positive redaction of common non-sensitive context words like 'failed', 'denied', 'required', 'header', etc.
The pattern is: r"\b(authorization|bearer)\b\s+\S+"
The intent is to catch HTTP Authorization header values like 'Authorization: Bearer <token>' or inline 'bearer <token>' patterns. But the 'authorization' alternative is too broad.
Possible fixes:
1. Remove 'authorization' from this pattern entirely since the 'bearer' alternative already catches the actual token in 'Authorization: Bearer <token>' format.
2. Make the 'authorization' alternative more specific, e.g. require a colon delimiter like 'authorization:\s+\S+' to only match header-like formats.
3. Add a negative lookahead for common non-sensitive words: 'authorization\s+(?!failed|denied|required|error|header|expired|invalid)\S+'.
Option 1 is simplest and most robust since Bearer already handles the main use case.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes part of issue #348 (support module correctness batch) and issue #351 (security hardening batch).
Issue: Exception cause chain could leak internal implementation details like connection strings, query fragments, or internal paths when formatted in error output.
Fix: In
PhloError._format_message(), only show the exception type name (e.g.,ValueError) instead of the full message (e.g.,ValueError: connection string with password=secret).Changes
File:
src/phlo/exceptions.pyf"Caused by: {type(self.cause).__name__}: {str(self.cause)}"tof"Caused by: {type(self.cause).__name__}"File:
tests/test_exceptions.pyTesting
Related Issues
Closes #348 item 2 (exception cause chain leak)
Closes #351 item 2 (Exception cause chain may leak internal implementation details)